The tests will be already failed given my tries before and there are many concerns about this. Also see and address the concerns in #9759.

This was more than a year ago, we should eventually upgrade..

I agree we should upgrade, but just changing the pom version will not work. I have an internal patch for this and I needed small changes in a few other places...

Which is, btw, a way of saying you should run dev/run-tests locally, at least, when changing a dependency.

I tried this in my local too. I didn't mean that we shouldn't but let's fix the test cases (or main codes) too and address other concerns.

I could not get the tests working locally. :-) Let me give it another try.

ok to test

Test build #92166 has finished for PR 21596 at commit ba454b0.

• This patch fails build dependency tests.
• This patch merges cleanly.
• This patch adds no public classes.

Please, make sure that performance doesn't degrade after upgrading Jackson. You can check that by JsonBenchmarks.

#20738

Bump jackson from 2.6.7&2.6.7.1 to 2.7.7
Jackson(>=2.7.7) fixes the possibility of missing tail data when the length of the value is in a range
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.7.7
https://github.com/FasterXML/jackson-core/issues/30

spark-shell:

val value = "x" * 3000
val json = s"""{"big": "$value"}"""
spark.sql("select length(get_json_object(\'"+json+"\','$.big'))" ).collect
res0: Array[org.apache.spark.sql.Row] = Array([2991])

expect result : 3000
actual result : 2991

@cxzl25, 2.7.x has a regression so we had to revert it back. Please see #9759.

@Fokko, mind adding the test described in #20738 here too?

Yes, let me add it and run the tests locally

I've pushed the changes, the tests are still running locally.

Test build #92209 has finished for PR 21596 at commit dfcc039.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

Hmm, I see a relevant failing test:

org.scalatest.exceptions.TestFailedException: "...430","name":"scope1"[,"parent":null]}" did not equal "...430","name":"scope1"[]}"

From the test core/src/test/scala/org/apache/spark/rdd/RDDOperationScopeSuite.scala:

  test("json de/serialization") {
    val scope1Json = scope1.toJson
    val scope2Json = scope2.toJson
    val scope3Json = scope3.toJson
    assert(scope1Json === s"""{"id":"${scope1.id}","name":"scope1"}""")
    assert(scope2Json === s"""{"id":"${scope2.id}","name":"scope2","parent":$scope1Json}""")
    assert(scope3Json === s"""{"id":"${scope3.id}","name":"scope3","parent":$scope2Json}""")
    assert(RDDOperationScope.fromJson(scope1Json) === scope1)
    assert(RDDOperationScope.fromJson(scope2Json) === scope2)
    assert(RDDOperationScope.fromJson(scope3Json) === scope3)
  }

It looks like it is inserting the null value ,"parent":null, which was omitted before. I would say that this is rather a design choice, than a bug.

Edit: I agree this field should not be rendered: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/rdd/RDDOperationScope.scala#L44

Ok, I've found the issue:

FasterXML/jackson-module-scala#325

Changing this to NON_ABSENT should fix this.

Test build #92229 has finished for PR 21596 at commit 9107328.

• This patch fails RAT tests.
• This patch merges cleanly.
• This patch adds no public classes.

Oops, a dangling file. Sorry for triggering so many builds guys.

Test build #92228 has finished for PR 21596 at commit 0d2e186.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

Test build #92231 has finished for PR 21596 at commit 1fd1d10.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

Test build #92230 has finished for PR 21596 at commit 0cb0ea5.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

When looking at the history server, we have a similar issue. From at the list command https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92231/testReport/org.apache.spark.deploy.history/HistoryServerSuite/stage_list_json/ :

I've formatted the json, expected:

[{
	"status": "COMPLETE",
	"stageId": 3,
	"attemptId": 0,
	"numTasks": 8,
	"numActiveTasks": 0,
	"numCompleteTasks": 8,
	"numFailedTasks": 0,
	"numKilledTasks": 0,
	"numCompletedIndices": 8,
	"executorRunTime": 162,
	"executorCpuTime": 0,
	"submissionTime": "2015-02-03T16:43:07.191GMT",
	"firstTaskLaunchedTime": "2015-02-03T16:43:07.191GMT",
	"completionTime": "2015-02-03T16:43:07.226GMT",
	"inputBytes": 160,
	"inputRecords": 0,
	"outputBytes": 0,
	"outputRecords": 0,
	"shuffleReadBytes": 0,
	"shuffleReadRecords": 0,
	"shuffleWriteBytes": 0,
	"shuffleWriteRecords": 0,
	"memoryBytesSpilled": 0,
	"diskBytesSpilled": 0,
	"name": "count at <console>:17",
	"details": "org.apache.spark.rdd.RDD.count(RDD.scala:910)\n$line19.$read$$iwC$$iwC$$iwC$$iwC.<init>(<console>:17)\n$line19.$read$$iwC$$iwC$$iwC.<init>(<console>:22)\n$line19.$read$$iwC$$iwC.<init>(<console>:24)\n$line19.$read$$iwC.<init>(<console>:26)\n$line19.$read.<init>(<console>:28)\n$line19.$read$.<init>(<console>:32)\n$line19.$read$.<clinit>(<console>)\n$line19.$eval$.<init>(<console>:7)\n$line19.$eval$.<clinit>(<console>)\n$line19.$eval.$print(<console>)\nsun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\nsun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)\nsun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\njava.lang.reflect.Method.invoke(Method.java:606)\norg.apache.spark.repl.SparkIMain$ReadEvalPrint.call(SparkIMain.scala:852)\norg.apache.spark.repl.SparkIMain$Request.loadAndRun(SparkIMain.scala:1125)\norg.apache.spark.repl.SparkIMain.loadAndRunReq$1(SparkIMain.scala:674)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:705)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:669)",
	"schedulingPool": "default",
	"rddIds": [6, 5],
	"accumulatorUpdates": [],
	"killedTasksSummary": {}
}, ... ]

found:

[{
	"status": "COMPLETE",
	"stageId": 3,
	"attemptId": 0,
	"numTasks": 8,
	"numActiveTasks": 0,
	"numCompleteTasks": 8,
	"numFailedTasks": 0,
	"numKilledTasks": 0,
	"numCompletedIndices": 8,
	"executorRunTime": 162,
	"executorCpuTime": 0,
	"submissionTime": "2015-02-03T16:43:07.191GMT",
	"firstTaskLaunchedTime": "2015-02-03T16:43:07.191GMT",
	"completionTime": "2015-02-03T16:43:07.226GMT",
	"failureReason": null,
	"inputBytes": 160,
	"inputRecords": 0,
	"outputBytes": 0,
	"outputRecords": 0,
	"shuffleReadBytes": 0,
	"shuffleReadRecords": 0,
	"shuffleWriteBytes": 0,
	"shuffleWriteRecords": 0,
	"memoryBytesSpilled": 0,
	"diskBytesSpilled": 0,
	"name": "count at <console>:17",
	"description": null,
	"details": "org.apache.spark.rdd.RDD.count(RDD.scala:910)\n$line19.$read$$iwC$$iwC$$iwC$$iwC.<init>(<console>:17)\n$line19.$read$$iwC$$iwC$$iwC.<init>(<console>:22)\n$line19.$read$$iwC$$iwC.<init>(<console>:24)\n$line19.$read$$iwC.<init>(<console>:26)\n$line19.$read.<init>(<console>:28)\n$line19.$read$.<init>(<console>:32)\n$line19.$read$.<clinit>(<console>)\n$line19.$eval$.<init>(<console>:7)\n$line19.$eval$.<clinit>(<console>)\n$line19.$eval.$print(<console>)\nsun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\nsun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)\nsun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\njava.lang.reflect.Method.invoke(Method.java:606)\norg.apache.spark.repl.SparkIMain$ReadEvalPrint.call(SparkIMain.scala:852)\norg.apache.spark.repl.SparkIMain$Request.loadAndRun(SparkIMain.scala:1125)\norg.apache.spark.repl.SparkIMain.loadAndRunReq$1(SparkIMain.scala:674)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:705)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:669)",
	"schedulingPool": "default",
	"rddIds": [6, 5],
	"accumulatorUpdates": [],
	"tasks": null,
	"executorSummary": null,
	"killedTasksSummary": {}
}, ... ]

We observe that the "failureReason": null, is listed, while it should have been omitted.

@Fokko, btw, I believe #21596 (comment) is a valid comment. I think it would be nicer if we execute this benchmark and update the results in the file, and push it into here.

BTW, I believe it's okay to use Jenkins resource if that's a faster way to resolve this issue in any event.

@HyukjinKwon Any idea if this has any change of getting merged? The 2.4 branch has been cut a while ago.

The number of merge conflicts are minimal in all the times I've rebased onto master. I think this should be done some time to bump the Jackson version, since old version of Jackson aren't compatible with the newer ones. Please let me know.

Test build #96330 has finished for PR 21596 at commit 44b8d1b.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

I think we're (at least i am) not clear on 2.5 vs 3.0 now (correct me if im mistaken). The change itself looks okay but I think it's targeted to 3.0.0 as discussed. I can take an action here when I'm clear on this.

Test build #96335 has finished for PR 21596 at commit 0b0b8fc.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

Test build #96377 has finished for PR 21596 at commit f4d294c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

Test build #96413 has finished for PR 21596 at commit 2bab06f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

Rebased onto master. @HyukjinKwon can we target this for 2.5?

Test build #96926 has finished for PR 21596 at commit b53c8f2.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

retest this please

Test build #96928 has finished for PR 21596 at commit b53c8f2.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

Merged to master.

It sounds like Jackson have an important behavior change that might impact us.

import com.fasterxml.jackson.annotation.JsonInclude.Include
import com.fasterxml.jackson.databind.{DeserializationFeature, ObjectMapper}
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper

val mapper = new ObjectMapper with ScalaObjectMapper
mapper.setSerializationInclusion(Include.NON_NULL)
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
mapper.registerModule(DefaultScalaModule)

println(mapper.writeValueAsString(Map("abc" -> null)))

Previously, it outputs {"abc":null}, but now it is {}, unless we explicitly set it to Include.ALWAYS. The upgrade looks risky to me.

@gatorsmile Are you completely sure about that? The Spark REST API null values are always omitted. Where is this behaviour different?

mapper.setSerializationInclusion(Include.NON_ABSENT):

scala> println(mapper.writeValueAsString(Map("abc" -> null)))
{}

mapper.setSerializationInclusion(Include.ALWAYS):

scala> println(mapper.writeValueAsString(Map("abc" -> null)))
{"abc":null}

If you're doing proper Scala and try to avoid the null's in general, with None in combination with mapper.setSerializationInclusion(Include.NON_ABSENT) you'll get:

scala> println(mapper.writeValueAsString(Map("abc" -> None)))
{"abc":null}